[SPARK-12941][SQL][MASTER] Spark-SQL JDBC Oracle dialect fails to map string datatypes to Oracle VARCHAR datatype #11306

thomastechs · 2016-02-22T18:08:51Z

What changes were proposed in this pull request?

This Pull request is used for the fix SPARK-12941, creating a data type mapping to Oracle for the corresponding data type"Stringtype" from dataframe. This PR is for the master branch fix, where as another PR is already tested with the branch 1.4

How was the this patch tested?

(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
This patch was tested using the Oracle docker .Created a new integration suite for the same.The oracle.jdbc jar was to be downloaded from the maven repository.Since there was no jdbc jar available in the maven repository, the jar was downloaded from oracle site manually and installed in the local; thus tested. So, for SparkQA test case run, the ojdbc jar might be manually placed in the local maven repository(com/oracle/ojdbc6/11.2.0.2.0) while Spark QA test run.

Added this new integration suite for creating the docker based test suite for the bug fix in SPARK-12941

Updated this pom.xml with the ojdbc jar related dependency to test the OracleIntegrationSuite related to SPARK-12941 bug fix

… suite addition Updated DockerJDBCIntegrationSuite.scala with the time out interval, as it is required to be atleast 200 sec for Oracle instance to be up

Updated OracleDialect.scala for [SPARK-12941] for the fix of adding a data type mapping to Oracle

…te addition Updated OracleIntegrationSuite.scala for [SPARK-12941] with Oracle Docker test cases

yhuai · 2016-02-22T19:07:50Z

ok to test

yhuai · 2016-02-22T19:21:54Z

@thomastechs Thank you for the PR. Since the jar is not in maven repo and you have tested it locally, how about we ignore the test? Let's keep the test but just ignore it. Let's also comment how to run the test. What do you think?

yhuai · 2016-02-22T21:35:54Z

ok to test

SparkQA · 2016-02-22T21:41:14Z

Test build #51684 has finished for PR 11306 at commit bde06fa.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

…es and ignore test case Updated the file with style check fixes and ignore test cases, as the ojdbc jar is not present in the maven.

Style check fixes

SparkQA · 2016-02-23T03:06:00Z

Test build #51717 has finished for PR 11306 at commit ca7ef4e.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

Updated the order of imports

thomastechs · 2016-02-23T03:18:37Z

@yhuai Thanks for the comments. I have put the ignore instead of test. The steps for the testing would be as follows.

Pull oracle 11g image - docker pull wnameless/oracle-xe-11g
Start docker - sudo service docker start
Download oracle 11g driver jar and put it in maven local repo. (com/oracle/ojdbc6/11.2.0.2.0/ojdbc6-11.2.0.2.0.jar)
Run spark test - ./build/sbt "test-only org.apache.spark.sql.jdbc.OracleIntegrationSuite"

SparkQA · 2016-02-23T03:21:17Z

Test build #51721 has finished for PR 11306 at commit 28a9e9f.

This patch fails build dependency tests.
This patch merges cleanly.
This patch adds no public classes.

thomastechs · 2016-02-23T03:34:32Z

@yhuai The style checks have been passed. Currently the build fails because the maven dependency oracle jar is not available, as expected. Suppose, we comment the oracle jar dependency in the pom file, still the build might fail in the beforeAll step where it tries to create a connection to Oracle docker using the ojdbc jar(which is not present in SparkQA env). So what would you suggest to proceed?

yhuai · 2016-02-23T04:45:15Z

docker-integration-tests/pom.xml

+      <artifactId>ojdbc6</artifactId>
+      <version>11.2.0.2.0</version>
+      <scope>test</scope>
+    </dependency>


Can you comment out this dependency since it is not in maven repo?

Updating this pom.xml to comment the changes related oracle ojdbc jar dependency as the maven repository does not contain this.

SparkQA · 2016-02-23T08:48:00Z

Test build #51741 has finished for PR 11306 at commit 87e3d7f.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

Updated the OracleIntegrationSuite.scala with the doc and override method

updated doc

SparkQA · 2016-02-23T09:59:47Z

Test build #51762 has finished for PR 11306 at commit 250591d.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

Updated style check fixes

thomastechs · 2016-02-23T12:18:16Z

jenkins test this please

yhuai · 2016-02-23T18:25:34Z

test this please

yhuai · 2016-02-23T18:44:54Z

test this please

SparkQA · 2016-02-23T21:03:06Z

Test build #51782 has finished for PR 11306 at commit d77d585.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

yhuai · 2016-02-24T00:16:28Z

docker-integration-tests/pom.xml

+        <version>11.2.0.2.0</version>
+        <scope>test</scope>
+      </dependency>
+    -->


Please also add comment at here to explain why we comment out this dependency.

yhuai · 2016-02-24T00:16:54Z

docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala

+
+import org.apache.spark.sql.test.SharedSQLContext
+import org.apache.spark.sql.types._
+import org.apache.spark.tags.DockerTest


Add a newline after this line.

Updated OracleIntegrationSuite.scala for [SPARK-12941] for the edits in the comments section

Added comments on why the oracle dependency was commented out

SparkQA · 2016-02-24T04:56:12Z

Test build #51837 has finished for PR 11306 at commit b3de7f4.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Updated DockerJDBCIntegrationSuite.scala- Reverted the timeout and interval parameter to 60 and 1. This would be required to be increased for Oracle based testing.

Updated the comments about the time interval change required in DockerJDBCIntegrationSuite.scala when testing with Oracle tests

SparkQA · 2016-02-24T08:15:40Z

Test build #51849 has finished for PR 11306 at commit 3f0f3b8.

This patch fails SparkR unit tests.
This patch merges cleanly.
This patch adds no public classes.

jayadevanmurali · 2016-02-24T09:25:12Z

@SparkQA Seems to be the build failed due kmeans error. I think this is not related to this PR. Could you please check

thomastechs · 2016-02-24T11:28:08Z

@yhuai @SparkQA Looks like the build failed due to some other unit test cases, not related to this PR
Could you please confirm?

thomastechs · 2016-02-25T01:22:52Z

@yhuai Could you initiate a retest please?

yhuai · 2016-02-25T01:36:38Z

test this please

SparkQA · 2016-02-25T03:57:36Z

Test build #51918 has finished for PR 11306 at commit 3f0f3b8.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

yhuai · 2016-02-26T06:37:49Z

docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala

+ * repository.
+ */
+@DockerTest
+class OracleIntegrationSuite extends DockerJDBCIntegrationSuite with SharedSQLContext {


How about we use Ignore tag to ignore the entire suite? So, we should not even try to connect to oracle.

oh nvm. Before will not be triggered since no test will run.

@yhuai the beforeall method will not execute because there is no valid test inside this class(already put ignore on top of test).

yhuai · 2016-02-26T06:49:35Z

Thanks. Merging to master.

… string datatypes to Oracle VARCHAR datatype ## What changes were proposed in this pull request? This Pull request is used for the fix SPARK-12941, creating a data type mapping to Oracle for the corresponding data type"Stringtype" from dataframe. This PR is for the master branch fix, where as another PR is already tested with the branch 1.4 ## How was the this patch tested? (Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests) This patch was tested using the Oracle docker .Created a new integration suite for the same.The oracle.jdbc jar was to be downloaded from the maven repository.Since there was no jdbc jar available in the maven repository, the jar was downloaded from oracle site manually and installed in the local; thus tested. So, for SparkQA test case run, the ojdbc jar might be manually placed in the local maven repository(com/oracle/ojdbc6/11.2.0.2.0) while Spark QA test run. Author: thomastechs <thomas.sebastian@tcs.com> Closes #11306 from thomastechs/master. (cherry picked from commit 8afe491) Signed-off-by: Yin Huai <yhuai@databricks.com>

thomastechs added 5 commits February 22, 2016 22:46

Adding new test suite for [SPARK-12941][SQL]

52ac008

Added this new integration suite for creating the docker based test suite for the bug fix in SPARK-12941

Update pom.xml for [SPARK-12941] docker test suite addition

b711688

Updated this pom.xml with the ojdbc jar related dependency to test the OracleIntegrationSuite related to SPARK-12941 bug fix

Update DockerJDBCIntegrationSuite.scala for [SPARK-12941] docker test…

c2c6d14

… suite addition Updated DockerJDBCIntegrationSuite.scala with the time out interval, as it is required to be atleast 200 sec for Oracle instance to be up

Update OracleDialect.scala for [SPARK-12941] docker test suite addition

54b2d4e

Updated OracleDialect.scala for [SPARK-12941] for the fix of adding a data type mapping to Oracle

Update OracleIntegrationSuite.scala for [SPARK-12941] docker test sui…

bde06fa

…te addition Updated OracleIntegrationSuite.scala for [SPARK-12941] with Oracle Docker test cases

thomastechs added 2 commits February 23, 2016 08:27

Update OracleIntegrationSuite.scala for [SPARK-12941] Style check fix…

0861d72

…es and ignore test case Updated the file with style check fixes and ignore test cases, as the ojdbc jar is not present in the maven.

Update OracleIntegrationSuite.scala for [SPARK-12941] Style check fixes

ca7ef4e

Style check fixes

Update OracleIntegrationSuite.scala for [SPARK-12941] Style check fixes

28a9e9f

Updated the order of imports

yhuai reviewed Feb 23, 2016
View reviewed changes

Update pom.xml

87e3d7f

Updating this pom.xml to comment the changes related oracle ojdbc jar dependency as the maven repository does not contain this.

thomastechs added 2 commits February 23, 2016 15:13

Update OracleIntegrationSuite.scala

7b12418

Updated the OracleIntegrationSuite.scala with the doc and override method

Update OracleIntegrationSuite.scala

250591d

updated doc

Update OracleIntegrationSuite.scala for [SPARK-12941] Style check fixes

d77d585

Updated style check fixes

yhuai reviewed Feb 24, 2016
View reviewed changes

thomastechs added 2 commits February 24, 2016 08:08

Update OracleIntegrationSuite.scala for [SPARK-12941] Updated Comments

7fcb16c

Updated OracleIntegrationSuite.scala for [SPARK-12941] for the edits in the comments section

Update pom.xml for [SPARK-12941] added comments

b3de7f4

Added comments on why the oracle dependency was commented out

thomastechs added 2 commits February 24, 2016 11:41

Update DockerJDBCIntegrationSuite.scala

345555f

Updated DockerJDBCIntegrationSuite.scala- Reverted the timeout and interval parameter to 60 and 1. This would be required to be increased for Oracle based testing.

Updated OracleIntegrationSuite.scala

3f0f3b8

Updated the comments about the time interval change required in DockerJDBCIntegrationSuite.scala when testing with Oracle tests

yhuai reviewed Feb 26, 2016
View reviewed changes

asfgit closed this in 8afe491 Feb 26, 2016

[SPARK-12941][SQL][MASTER] Spark-SQL JDBC Oracle dialect fails to map string datatypes to Oracle VARCHAR datatype #11306

[SPARK-12941][SQL][MASTER] Spark-SQL JDBC Oracle dialect fails to map string datatypes to Oracle VARCHAR datatype #11306

Uh oh!

Conversation

thomastechs commented Feb 22, 2016

What changes were proposed in this pull request?

How was the this patch tested?

Uh oh!

yhuai commented Feb 22, 2016

Uh oh!

yhuai commented Feb 22, 2016

Uh oh!

yhuai commented Feb 22, 2016

Uh oh!

SparkQA commented Feb 22, 2016

Uh oh!

SparkQA commented Feb 23, 2016

Uh oh!

thomastechs commented Feb 23, 2016

Uh oh!

SparkQA commented Feb 23, 2016

Uh oh!

thomastechs commented Feb 23, 2016

Uh oh!

yhuai Feb 23, 2016

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Feb 23, 2016

Uh oh!

SparkQA commented Feb 23, 2016

Uh oh!

thomastechs commented Feb 23, 2016

Uh oh!

yhuai commented Feb 23, 2016

Uh oh!

yhuai commented Feb 23, 2016

Uh oh!

SparkQA commented Feb 23, 2016

Uh oh!

yhuai Feb 24, 2016

Choose a reason for hiding this comment

Uh oh!

yhuai Feb 24, 2016

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Feb 24, 2016

Uh oh!

SparkQA commented Feb 24, 2016

Uh oh!

jayadevanmurali commented Feb 24, 2016

Uh oh!

thomastechs commented Feb 24, 2016

Uh oh!

thomastechs commented Feb 25, 2016

Uh oh!

yhuai commented Feb 25, 2016

Uh oh!

SparkQA commented Feb 25, 2016

Uh oh!

yhuai Feb 26, 2016

Choose a reason for hiding this comment

Uh oh!

yhuai Feb 26, 2016

Choose a reason for hiding this comment

Uh oh!

jayadevanmurali Feb 26, 2016

Choose a reason for hiding this comment

Uh oh!

yhuai commented Feb 26, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants